On Tuesday, August 20th, 2024 at 3:12 AM, Andy Yan <andyshrk@xxxxxxx> wrote:
>
> Hi Piotr，
Hi Andy!
> > +static int vop2_crtc_atomic_check_gamma(struct vop2_video_port *vp,
> > + struct drm_crtc *crtc,
> > + struct drm_atomic_state *state,
> > + struct drm_crtc_state *crtc_state)
> > +{
> > + struct vop2 *vop2 = vp->vop2;
> > + unsigned int len;
> > +
> > + if (!vp->vop2->lut_regs || !crtc_state->color_mgmt_changed ||
> > + !crtc_state->gamma_lut)
> > + return 0;
> > +
> > + len = drm_color_lut_size(crtc_state->gamma_lut);
> > + if (len != crtc->gamma_size) {
> > + DRM_DEBUG_KMS("Invalid LUT size; got %d, expected %d\n",
> > + len, crtc->gamma_size);
> > + return -EINVAL;
> > + }
> > +
> > + // trigger full modeset only when SoC is 356x
> > + if (!crtc_state->mode_changed && (vop2->data->soc_id == 3566 ||
> > + vop2->data->soc_id == 3568)) {
> > + int ret;
> > +
> > + crtc_state->mode_changed = true;
> > + state->allow_modeset = true;
>
>
>
>
> We don't need to trigger a modeset here. We just need to disable dsp_lut befor we write gamma lut data for rk3566/8.
Formerly my patch didn't trigger a modeset. Though Daniel Stone in his
reply to v3[1] suggested it as the clean way to handle RK356x case[2],
quote, "it would probably be better to set mode_changed when the colour
management configuration changes". Let's wait for his reply to this
version of the patch, perhaps he meant something different or not exactly
what I did.
[1] https://lore.kernel.org/linux-rockchip/CAPj87rOM=j0zmuWL9frGKV1xzPbJrk=Q9ip7F_HAPYnbCqPouw@xxxxxxxxxxxxxx/
[2] https://lore.kernel.org/linux-rockchip/TkgKVivuaLFLILPY-n3iZo_8KF-daKdqdu-0_e0HP-5Ar_8DALDeNWog2suwWKjX7eomcbGET0KZe7DlzdhK2YM6CbLbeKeFZr-MJzJMtw0=@proton.me/
Best Regards, Piotr Zalewski
