Re: [PATCH v3] rockchip/drm: vop2: add support for gamma LUT

[Daniel Stone]

Hi Piotr,

On Wed, 24 Jul 2024 at 23:06, Piotr Zalewski <pZ010001011111@xxxxxxxxx> wrote:
> Add support for gamma LUT in VOP2 driver. The implementation is based on
> the one found in VOP driver and modified to be compatible with VOP2. Blue
> and red channels in gamma LUT register write were swapped with respect to
> how gamma LUT values are written in VOP. Write of the current video port id
> to VOP2_SYS_LUT_PORT_SEL register was added before the write of DSP_LUT_EN
> bit. Gamma size is set and drm color management is enabled for each video
> port's CRTC except ones which have no associated device. Tested on RK3566
> (Pinetab2).

Thanks a lot for doing this!

> +static void vop2_crtc_gamma_set(struct vop2 *vop2, struct drm_crtc *crtc,
> +                               struct drm_crtc_state *old_state)
> +{
> +       [...]
> +
> +       vop2_lock(vop2);
> +       vop2_crtc_write_gamma_lut(vop2, crtc);
> +       vop2_writel(vp->vop2, RK3568_LUT_PORT_SEL, vp->id);
> +
> +       vop2_vp_dsp_lut_enable(vp);
> +
> +       vop2_cfg_done(vp);
> +       vop2_unlock(vop2);
> +}
>
> @@ -2060,6 +2159,9 @@ static void vop2_crtc_atomic_enable(struct drm_crtc *crtc,
>         drm_crtc_vblank_on(crtc);
>
>         vop2_unlock(vop2);
> +
> +       if (crtc->state->gamma_lut)
> +               vop2_crtc_gamma_set(vop2, crtc, old_state);
>  }

In the atomic_enable callback, we are already holding the VOP lock,
and waiting to set cfg_done etc - we then do it all over again here.
This should all be atomic, so that we configure the LUT whilst doing
the setup, and then only call cfg_done once, to avoid showing the user
intermediate states which only later converge on the desired final
state.

Cheers,
Daniel