Hi Tomi, Thank you for the patch.
On Mon, Sep 02, 2019 at 03:53:56PM +0300, Tomi Valkeinen wrote: > From: Jyri Sarha <[email protected]> > > Implement CTM color management property for OMAP CRTC using DSS > overlay manager's Color Phase Rotation matrix. The CPR matrix does not > exactly match the CTM property documentation. On DSS the CPR matrix is > applied after gamma table look up. However, it seems stupid to add a > custom property just for that. In that case the DRM documentation should be updated to mention that both options are allowed. > Signed-off-by: Jyri Sarha <[email protected]> > Signed-off-by: Tomi Valkeinen <[email protected]> > --- > drivers/gpu/drm/omapdrm/omap_crtc.c | 39 +++++++++++++++++++++++++++-- > 1 file changed, 37 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c > b/drivers/gpu/drm/omapdrm/omap_crtc.c > index 3c5ddbf30e97..d63213dd7d83 100644 > --- a/drivers/gpu/drm/omapdrm/omap_crtc.c > +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c > @@ -391,6 +391,32 @@ static void omap_crtc_manual_display_update(struct > work_struct *data) > } > } > > +static s16 omap_crtc_S31_32_to_s2_8(s64 coef) > +{ > + uint64_t sign_bit = 1ULL << 63; > + uint64_t cbits = (uint64_t) coef; s/uint64_t/u64/ for both lines as we're dealing with kernel code. And there's no need for a space before coef. > + s16 ret = clamp_val(((cbits & ~sign_bit) >> 24), 0, 0x1FF); > + > + if (cbits & sign_bit) > + ret = -ret; > + > + return ret; Can't this be simplified to s16 ret = (coef >> 24) & 0x1ff; return coef < 0 ? -ret : ret; > +} > + > +static void omap_crtc_cpr_coefs_from_ctm(const struct drm_color_ctm *ctm, > + struct omap_dss_cpr_coefs *cpr) > +{ > + cpr->rr = omap_crtc_S31_32_to_s2_8(ctm->matrix[0]); > + cpr->rg = omap_crtc_S31_32_to_s2_8(ctm->matrix[1]); > + cpr->rb = omap_crtc_S31_32_to_s2_8(ctm->matrix[2]); > + cpr->gr = omap_crtc_S31_32_to_s2_8(ctm->matrix[3]); > + cpr->gg = omap_crtc_S31_32_to_s2_8(ctm->matrix[4]); > + cpr->gb = omap_crtc_S31_32_to_s2_8(ctm->matrix[5]); > + cpr->br = omap_crtc_S31_32_to_s2_8(ctm->matrix[6]); > + cpr->bg = omap_crtc_S31_32_to_s2_8(ctm->matrix[7]); > + cpr->bb = omap_crtc_S31_32_to_s2_8(ctm->matrix[8]); > +} > + > static void omap_crtc_write_crtc_properties(struct drm_crtc *crtc) > { > struct omap_drm_private *priv = crtc->dev->dev_private; > @@ -402,7 +428,16 @@ static void omap_crtc_write_crtc_properties(struct > drm_crtc *crtc) > info.default_color = 0x000000; > info.trans_enabled = false; > info.partial_alpha_enabled = false; > - info.cpr_enable = false; > + > + if (crtc->state->ctm) { > + struct drm_color_ctm *ctm = > + (struct drm_color_ctm *) crtc->state->ctm->data; > + > + info.cpr_enable = true; > + omap_crtc_cpr_coefs_from_ctm(ctm, &info.cpr_coefs); As an optimisation it would be nice to only write the coefficients when they actually change. That could be implemented on top of this series. > + } else { > + info.cpr_enable = false; > + } > > priv->dispc_ops->mgr_setup(priv->dispc, omap_crtc->channel, &info); > } > @@ -836,7 +871,7 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev, > if (priv->dispc_ops->mgr_gamma_size(priv->dispc, channel)) { > unsigned int gamma_lut_size = 256; > > - drm_crtc_enable_color_mgmt(crtc, 0, false, gamma_lut_size); > + drm_crtc_enable_color_mgmt(crtc, 0, true, gamma_lut_size); > drm_mode_crtc_set_gamma_size(crtc, gamma_lut_size); > } > -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
